fix(api): owner-scope caller-supplied FKs in eval-cases + interaction…#81
Merged
Conversation
… (L4, L5) Audit findings L4/L5: both routes stored caller-supplied document/chunk ids (and free-text labels) without verifying ownership, letting a client inject arbitrary or cross-owner references into the miss-promotion / eval pipeline. - search/interaction: gate clicked_document_id, top_files, top_chunk_ids and candidate_labels on a documents.owner_id check; the query telemetry is still recorded. - eval-cases: verify expectedDocumentId (and expectedChunkId via its document) belong to the caller; null them otherwise. Added a test proving an unowned expectedDocumentId is dropped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens two API routes against tenant-crossing reference injection by ensuring caller-supplied document/chunk references are only persisted when they belong to the authenticated owner, reducing the risk of polluting the eval/miss-promotion pipeline with cross-owner pointers.
Changes:
/api/search/interaction: gate persistedclicked_document_id-related telemetry (and derived labels/top_* fields) behind adocuments.owner_id = user.idcheck./api/eval-cases: verifyexpectedDocumentIdis owned by the caller (andexpectedChunkIdvia its parent document) and null them otherwise before inserting intorag_query_misses.- Add a test asserting an unowned
expectedDocumentIdis dropped.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/eval-cases-route.test.ts | Adds coverage ensuring unowned expectedDocumentId is not persisted. |
| src/app/api/search/interaction/route.ts | Adds ownership gating before attributing clicked doc + label/top_* telemetry to rag_query_misses. |
| src/app/api/eval-cases/route.ts | Adds ownership verification for caller-supplied expected_* foreign keys before insert. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+47
to
+50
| clicked_document_id: documentOwned ? body.documentId : null, | ||
| clicked_chunk_id: documentOwned ? (body.chunkId ?? null) : null, | ||
| top_files: documentOwned && body.fileName ? [body.fileName] : [], | ||
| top_chunk_ids: documentOwned && body.chunkId ? [body.chunkId] : [], |
Comment on lines
+101
to
+105
| let expectedChunkId: string | null = null; | ||
| if (parsed.data.expectedChunkId) { | ||
| const { data: chunkRow } = await supabase | ||
| .from("document_chunks") | ||
| .select("document_id") |
4 tasks
cursor Bot
pushed a commit
that referenced
this pull request
Jul 25, 2026
…1215) * test(#30): generalize the alias-slot disjointness guard The existing contracts pin the admission/discharge pair by name, so a future multi-slot case -- or a widening of a different alias tier -- could reintroduce the #30 defect without any test going red. Drive both invariants off the real eval cases instead: no expectation of a multi-slot case may share alias values with another slot of the same case, and no single document may satisfy every slot of any multi-slot case. The second holds independently of the alias tables, so it stays meaningful if the tables are widened again. RAG impact: no retrieval behaviour change -- test-only. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(issues): open #81 for the PR #1196 alias conflict PR #1196 re-adds two admission-to-discharge titles to the wide-tier AdmissionCommunityPts list, which would revert the #30 tightening. Its other conflicts include protected RAG surfaces, so record how to reconcile it rather than leaving that in chat context. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
BigSimmo
added a commit
that referenced
this pull request
Jul 25, 2026
* issues: close #81 — PR #1196 closed, alias regression now contract-guarded Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * chore: re-run required checks after bot branch sync Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
… (L4, L5)
Audit findings L4/L5: both routes stored caller-supplied document/chunk ids (and free-text labels) without verifying ownership, letting a client inject arbitrary or cross-owner references into the miss-promotion / eval pipeline.
Summary
Verification
npm run verify:cheapnpm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changednpm run verify:releasebefore release or handoff confidence claimsnpm run format:checknpm run check:production-readinesswhen clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changedClinical Governance Preflight
Complete this section when the change touches ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output.
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes